Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite of 'Adjust correction history' condition #5685

Closed

Conversation

pb00068
Copy link
Contributor

@pb00068 pb00068 commented Nov 20, 2024

Current condition is IMO convoluted and hard to understand because of several negations.
Also added 2 comments to make the concept behind the condition better understandable.

No functional change

bench: 840721

Copy link

clang-format 18 needs to be run on this PR.
If you do not have clang-format installed, the maintainer will run it when merging.
For the exact version please see https://packages.ubuntu.com/noble/clang-format-18.

(execution 11935135151 / attempt 1)

@pb00068 pb00068 changed the title Rewrite Adjust correction history condition in a more human readable way Rewrite of 'Adjust correction history' condition Nov 20, 2024
&& !(!bestMove && bestValue >= ss->staticEval))
if (!ss->inCheck && !(bestMove && pos.capture(bestMove))
&& ((bestValue < ss->staticEval && bestValue < beta ) // negative correction & no fail high
|| (bestValue > ss->staticEval && bestMove))) // positive correction & no fail low
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Do not apply negative correction when failing high
// Do not apply positive correction when failing low

Do you think this might be a clearer rewording of the comments above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Yes, I like it.
But before changing something I wait for further reactions (maintainers will decide).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i prefer the initial version, short and concise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants